-
Notifications
You must be signed in to change notification settings - Fork 324
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Lint Issues- Testing out addition of Check Destroy in some Acceptance Tests + other refactoring #998
Conversation
Can you remove the AT001 check from the provider lint check ignores: terraform-provider-gitlab/GNUmakefile Line 23 in 9c09272
|
This pull request has merge conflicts. Please rebase your branch onto |
45a79cd
to
60d19c4
Compare
# Conflicts: # internal/provider/resource_gitlab_project_test.go
Conflicts are resolved. Thank you! 😀 |
@timofurrer It appears that AT001 is different from XAT001 in the linter. Turning off XAT001 makes a massive amount of separate lint issues so leaving it for now. |
@timofurrer I combed over the test case and am not sure why the Acceptance Test is failing. If you could take a look I think this is the last thing |
@armsnyder If you have a chance as well to take a look the issue is not apparent to me why it is failing. |
Hi @Shocktrooper -- I couldn't tell what the issue was either. I assume it has to do with the new function you added to render out the Config field in each TestStep. Maybe try removing the TestSteps one by one, and running a targeted test to catch where the problem lies. I also opened this PR to make the error more helpful in the future, not that it will help us now. 😄 hashicorp/terraform-plugin-sdk#925 |
@armsnyder I figured out what steps at least. Apparently having a completely blank resource config could be the issue from what I can tell or it was the way the anonymous function was returned in the other Not Shared check function. After looking at the test again I didn't think it was needed either because we have a check destroy function so it should all be working now :) This is all ready for review again |
@Shocktrooper @armsnyder I've actually noticed that in other providers test where the have something like |
@timofurrer interesting do you have an example of this in action as I am unsure how that would be different than what I replaced in my last commit. Also is there a point to blank configs when you have a check destroy function. |
b7ea5da
to
a5d33a6
Compare
a5d33a6
to
f3e1194
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🎉
This functionality has been released in v3.14.0 of the Terraform GitLab Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue. Thank you! |
Resolves #433